Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created a testcase for auto primary content #460

Draft
wants to merge 7 commits into
base: dev
Choose a base branch
from
Draft

Conversation

AvantikaM5
Copy link
Contributor

No description provided.

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Mar 14, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 16715a4
Status: ✅  Deploy successful!
Preview URL: https://df4b1211.emseapi.pages.dev
Branch Preview URL: https://almp-620.emseapi.pages.dev

View logs

@codecov
Copy link

codecov bot commented Mar 14, 2023

Codecov Report

Merging #460 (e348ac4) into dev (264dbd8) will not change coverage.
The diff coverage is n/a.

❗ Current head e348ac4 differs from pull request most recent head 7caa1aa. Consider uploading reports for the commit 7caa1aa to get more accurate results

@@           Coverage Diff           @@
##              dev     #460   +/-   ##
=======================================
  Coverage   91.76%   91.76%           
=======================================
  Files          33       33           
  Lines        4834     4834           
  Branches       32       32           
=======================================
  Hits         4436     4436           
  Misses        384      384           
  Partials       14       14           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Collaborator

@chef-danny-d chef-danny-d left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall one test case is not enough to test the whole feature. Please rethink you approach and submit your changes.

coll.map((c) => {
c.lessons.map((lesson) => {
if (lesson.content.length == 1)
expect(lesson.content[0].primary).toBe(true);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are expecting the lesson's first content to be primary if the array length is 1.

  • What is this test cases checking if the content length is more then 1?
  • Is this test case covering creating content with a primary conflict?
  • Is this test case covering updating content to be primary where there already is a primary in the Array?

@chef-danny-d chef-danny-d marked this pull request as draft April 12, 2023 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants